Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix strfry x-forwarded-for ip by using network_mode: host #142

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dcadenas
Copy link
Contributor

@dcadenas dcadenas commented Jan 8, 2025

While investigating the events service queue, I noticed that events pushed from it were being rejected due to rate limits. The issue occurred because strfry was only seeing Traefik’s internal IP instead of the actual client IP. To resolve this, I added network_mode: "host" to the Traefik service configuration. With this change, strfry now correctly sees the real client IP, and events are no longer rejected.

Caveats:
I acknowledge that using network_mode: "host" is a more open configuration, but given the nature of our relay, which is designed to be highly exposed, this should not introduce significant risks. I also explored alternatives to expose the X-Forwarded-For header without enabling host networking but could not find a suitable solution.

@mplorentz
Copy link
Member

I'm ok with this, but I'm curious if @nbenmoody is concerned.

@mplorentz
Copy link
Member

Bump @nbenmoody

@nbenmoody
Copy link
Collaborator

nbenmoody commented Jan 29, 2025

Ack, sorry for being so tardy here. No excuses, just lost track of this one. If I'm understanding correctly, the rate-limiting is coming from stryfry, correct? I'm completely unfamiliar with the rate-limiting configuration on stryfry. I'd of course opt for more granular configuration over exposure, but running a container the host network isn't in itself a security issue, it would just require us to take a detailed pass through everything that is possibly exposed that way.

Host networking just removes the network isolation layer between the container and the host, so the container is exposed the same way any application that binds a port on the host would be. With something like Traefik, that is well-supported and widely used, and updated frequently, I'd say the risk is minimal (so long as we make sure we know exactly what services traefik will be exposing on what ports). I wouldn't run any random third-party containers that way, because they have full access to localhost, and essentially the whole network stack on the server.

So, in shorter words: I think this is probably fine, but we will want to make sure Traefik is only listening on port 443 (or 80 with redirect), and that the server's firewall has all other ports disabled (except for 22, or whatever ssh is listening on). We will also want to make sure we aren't accidentally exposing the Traefik dashboard, or any admin endpoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants